Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent publication with unsupported credential types #351

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

pcanilho
Copy link
Contributor

@pcanilho pcanilho commented Jul 5, 2023

fixes: #347

What changed?

Unsupported credential types are now explicitly made incompatible with the github-checks-plugin to avoid downstream unexpected exceptions such as the one reported here.

With this change, the supported credential types are:

  • GitHubAppCredentials
  • VaultUsernamePasswordCredentialImpl

If an unsupported credential type is provided, an early return is done to avoid having to handle downstream exceptions caused by incompatible types. This means that all pipeline console log entries will be suppressed if the pipeline has not been configured with compatible credentials.

Testing done

Procedure
As extracted from here, I have ran the following command to both build & run the test suite:

$ mvn -Penable-jacoco clean verify -B -V -ntp

A regression test was also carried out in our internal Jenkins controller to assert that previous functionality based on the above described credential types is left unaffected.

Submitter checklist

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess ok

@timja timja requested review from uhafner and XiongKezhi July 5, 2023 16:07
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Merging #351 (0e877c2) into master (983fed5) will decrease coverage by 0.22%.
The diff coverage is 33.33%.

@@             Coverage Diff              @@
##             master     #351      +/-   ##
============================================
- Coverage     72.02%   71.80%   -0.22%     
  Complexity      162      162              
============================================
  Files            16       16              
  Lines           529      532       +3     
  Branches         50       51       +1     
============================================
+ Hits            381      382       +1     
- Misses          123      124       +1     
- Partials         25       26       +1     
Impacted Files Coverage Δ
...s/plugins/checks/github/GitHubChecksPublisher.java 94.54% <33.33%> (-3.54%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@meiswjn
Copy link

meiswjn commented Jul 7, 2023

@meiswjn
Copy link

meiswjn commented Aug 22, 2023

Can this be merged? :) @timja

@timja timja merged commit bee03a0 into jenkinsci:master Aug 22, 2023
14 of 16 checks passed
@timja timja added the bug Something isn't working label Aug 22, 2023
@timja
Copy link
Member

timja commented Aug 22, 2023

heh I was giving @uhafner time to review as he seemed to get annoyed last time I merged same day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken using SSH credentials with this plugin
3 participants